Core: Fix operator[] for typed dictionaries
#96797
Merged
+84
−46
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dalexeev
reviewed
Sep 10, 2024
modules/gdscript/tests/scripts/analyzer/errors/typed_dictionary_assign_differently_typed_key.gd
Outdated
Show resolved
Hide resolved
Repiteo
force-pushed
the
core/typed-dictionary-bracket-fix
branch
2 times, most recently
from
3e52612
to
3d79d8b
Compare
dalexeev
reviewed
Sep 11, 2024
...les/gdscript/tests/scripts/runtime/errors/typed_dictionary_assign_differently_typed_value.gd
Outdated
Show resolved
Hide resolved
modules/gdscript/tests/scripts/analyzer/errors/typed_dictionary_assign_differently_typed_key.gd
Outdated
Show resolved
Hide resolved
Repiteo
force-pushed
the
core/typed-dictionary-bracket-fix
branch
4 times, most recently
from
e2a9e04
to
57e3d4e
Compare
dalexeev
approved these changes
Sep 12, 2024
Repiteo
force-pushed
the
core/typed-dictionary-bracket-fix
branch
2 times, most recently
from
783c8c0
to
962f02d
Compare
dalexeev
reviewed
Sep 12, 2024
core/variant/dictionary.cpp
Outdated
Comment on lines
86
to
87
| /// @warning This operator does not validate the value type. For scripting/extensions this is done | ||
| /// in `variant_setget.cpp`. Consider using `set()` if the data might be invalid. |
There was a problem hiding this comment.
(Just a note) Perhaps we should stick to one style for comment keywords.
Occurrences
core/io/packet_peer.cpp: 1
302: 2: //warning may lose packets
core/math/transform_2d.h: 2
42: 2: // Warning #1: basis of Transform2D is stored differently from Basis. In terms of columns array, the basis matrix looks like "on paper":
50: 2: // Warning #2: 2D be aware that unlike 3D code, 2D code uses a left-handed coordinate system: Y-axis points down,
editor/plugins/tiles/tile_set_atlas_source_editor.h: 1
180: 3: // Warning: keep in this order.
modules/gdscript/gdscript_compiler.cpp: 1
2634: 1: // Warning: this function cannot initiate compilation of other classes, or it will result in cyclic dependency issues.
modules/gdscript/gdscript_parser.cpp: 1
4319: 3: // WARNING: Do not merge with the previous `if` because there `!=`, not `==`!
modules/mono/glue/GodotSharp/GodotSharp/Core/GD.cs: 1
29: 9: /// Warning: Deserialized object can contain code which gets executed. Do not use this
platform/windows/display_server_windows.cpp: 1
3965: 2: // WARNING: we get called with events before the window is registered in our collection
platform/windows/gl_manager_windows_native.cpp: 1
455: 2: // WARNING: p_window_id is an eternally growing integer since popup windows keep coming and going
servers/physics_2d/godot_body_pair_2d.cpp: 1
165: 1: // Warning: the way velocity is adjusted down to cause a collision means the momentum will be weaker than it should for a bounce!
servers/physics_2d/godot_step_2d.cpp: 2
254: 2: // Warning: This doesn't run on threads, because it involves thread-unsafe processing.
261: 2: // Warning: _solve_island modifies the constraint islands for optimization purpose,
servers/physics_3d/godot_body_pair_3d.cpp: 1
165: 1: // Warning: the way velocity is adjusted down to cause a collision means the momentum will be weaker than it should for a bounce!
servers/physics_3d/godot_step_3d.cpp: 2
358: 2: // Warning: This doesn't run on threads, because it involves thread-unsafe processing.
365: 2: // Warning: _solve_island modifies the constraint islands for optimization purpose,
servers/rendering/renderer_rd/shaders/effects/ssao.glsl: 2
53: 54: #define SSAO_DEPTH_MIPS_ENABLE_AT_QUALITY_PRESET (2) // !!warning!! the MIP generation on the C++ side will be enabled on quality preset 2 regardless of this value, so if changing here, change
56: 1: // !!warning!! the edge handling is hard-coded to 'disabled' on quality level 0, and enabled above, on the C++ side; while toggling it here will work for
servers/rendering/renderer_rd/shaders/effects/ssil.glsl: 1
52: 1: // !!warning!! the edge handling is hard-coded to 'disabled' on quality level 0, and enabled above, on the C++ side; while toggling it here will work for
There was a problem hiding this comment.
I'm personally a big fan of the "proper" syntax implemented in gdextension_interface.h, though that only works in a docstrings & is primarily suited for exposed/binding contexts. I'm down for a universal WARNING: preface, akin to TODO: & FIXME: , though that'll need to be specified in the documentation.
Repiteo
force-pushed
the
core/typed-dictionary-bracket-fix
branch
from
962f02d
to
e8017e5
Compare
Repiteo
force-pushed
the
core/typed-dictionary-bracket-fix
branch
from
e8017e5
to
b3d7960
Compare
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
operator[]#96772Caused by an erroneous regression when rebasing the typed dictionary PR. Reimplements type checking and passing
_p->typed_fallbackto erroneous keys when usingoperator[]on dictionaries.This is technically a partial fix, because it seems the untyped value assignment via
operator[]is something that can happen regardless. However, fixing that will probably be a much more involved change, as the passed value isn't linked to the typed dictionary in isolation.